GitHub rejected REQUEST_CHANGES because the authenticated account owns this pull request. The findings below are blocking from a maintainer review perspective. I would not merge this until the blocking issues above are addressed.

Fresh review of the current net diff: I did not find blocking issues. The PEPS scaffold is now kept out of exports/public API docs, and the restored Val backend dispatch preserves the extension hook. I left two nonblocking comments on test coverage and documentation wording.

Reviewed the current net diff against feature/issue-37-backend-interface. No blocking or nonblocking inline issues found. The PEPS scaffolding remains internal, solve_ising is covered as public API, and the updated tests cover the newly public behavior and internal boundary sufficiently for this PR.

GitHub would not let me submit APPROVE because this account is the PR author: "Review Can not approve your own pull request".

Tests run locally:

• git diff --check origin/feature/issue-37-backend-interface...HEAD
• julia --project=. -e "using Pkg; Pkg.test()"
• julia --project=. -e "using Test, Random, LinearAlgebra, TenSolver; include("test/utils.jl"); include("test/backend.jl"); include("test/peps_backend.jl")"
• julia +1.12 --project=. -e "using Test, Random, LinearAlgebra, TenSolver; include("test/utils.jl"); include("test/backend.jl"); include("test/peps_backend.jl")"
• julia --project=. targeted solve_ising exact-energy probe
• julia --project=docs/ -e "using Pkg; Pkg.develop(PackageSpec(path=pwd())); Pkg.instantiate()"
• julia --project=docs/ docs/make.jl local

The optional SpinGlassPEPS extension path still cannot be exercised locally because SpinGlassNetworks/SpinGlassEngine/SpinGlassTensors cannot currently resolve with this package dependency stack; the PR documents that limitation and keeps the PEPS backend internal.

Merge recommendation: merge-ready from this review pass.

Senior maintainer review. Note: I am the PR author, so GitHub only permits a COMMENT event from me; a formal approval or change-request must come from another maintainer. No blocking issues; one substantive Question for the reviewer below.

Diff verification: confirmed against the stacked base feature/issue-37-backend-interface (not main). merge-base 009bcd1; git diff <merge-base>..HEAD --stat = 10 files, +646 / -5, matching the PR's reported totals.

Issue intent (#38): add an optional SpinGlassPEPS-backed path without making SpinGlassPEPS a hard dependency; reuse the #42 conversions; decode back to Boolean and report the objective in TenSolver's convention; keep DMRG default; guard tests so absence of the heavy dep / Julia 1.10 does not break core CI. The PR satisfies all of these.

Optionality — PASS: the three SpinGlass packages are in [weakdeps] + [extensions] (TenSolverSpinGlassPEPSExt), not in [deps]. julia = "1.10" is left unchanged (no silent breaking bump); the extension mechanism is valid on 1.10.

Julia-compat reality (Question, not a blocker): I confirmed empirically on Julia 1.10.11 that SpinGlassNetworks@1.4/SpinGlassTensors@1.3 are unresolvable (resolver: "restricted by julia compatibility ... no versions left"). So on the only CI-able Julia (1.10) the weakdeps can never install, the extension never loads, and ordinary users are unaffected — but the entire SpinGlassPEPS bridge is therefore dead/untested-on-CI until TenSolver raises compat to 1.11. The PR is honest about this and keeps the PEPS types internal/unexported. For the reviewer to decide: merge now as the "experimental scaffolding" #38 explicitly permits (option 3), or defer activation until compat is raised to 1.11. Not blocking under #38's stated acceptance.

Bridge correctness (static; heavy dep not runnable here) — looks correct: _minimize(::PEPSBackend, ...) reuses qubo_to_ising; the reported objective is recomputed independently in TenSolver's convention via _decoded_records (decode Potts -> spins -> spin_to_bool -> ising_energy), not trusting SpinGlassPEPS's raw_energy (kept as metadata) — this sidesteps any sign/offset mismatch between the libraries. _ising_instance maps diagonal=biases, off-diag=couplings, consistent with the upper-triangular canonical J. The DMRG solve_ising round trip is verified by the new passing test.

Default unchanged — PASS. Tests guarded — PASS: test/peps_backend.jl gates the extension test on Base.find_package for all three components and uses @test_skip (observed as "Broken 1", i.e. skipped, on 1.10); the always-run "PEPS backend core" tests only the available types/validation.

Nonblocking

1. src/solution.jl narrows Base.in(bs, psi) to Base.in(bs::AbstractVector, psi). All in-repo callers pass vectors and tests pass, but this is a subtle public-API signature narrowing — confirm intended.
2. src/TenSolver.jl adds export Solution (previously unexported). Purely additive/non-breaking; PEPSSolution/PEPSBackend/SquareGrid/KingGrid are intentionally not exported.
3. ext/...Ext.jl _metadata reads SpinGlassEngine Solution fields (energies/probabilities/largest_discarded_probability); plausible but unverifiable without the dep — confirm against SpinGlassEngine 1.6 when the path is first exercised.

Tests run (Julia 1.10.11):

• julia --project=. -e 'using Pkg; Pkg.instantiate(); Pkg.test()' — passed (Testing TenSolver tests passed): Backend interface 25/25, PEPS backend core 34/34, Optional SpinGlassPEPS extension = 1 Broken (skipped as expected), QUBODrivers 128/128, Aqua 11/11, VRP/HDF5/Doctests pass.
• Adversarial install: Pkg.add(SpinGlassNetworks@1.4) on fresh 1.10 -> resolve failed with explicit julia-compat exclusion (confirms uninstallable on 1.10; expected, not a PR failure).

Merge readiness: No blocking issues. The PR honors #38: optionality via package extension (not [deps]), DMRG default preserved, conversions reused, objective decoded into TenSolver's Boolean convention, tests properly guarded, core suite green on 1.10. The substantive caveat is the Question above: the bridge is uninstallable/untested on CI Julia and remains so until compat is raised to 1.11; merge as documented scaffolding or defer, and track a follow-up to exercise it under 1.11. Stacked on #43 and must merge after it. The formal verdict must come from another maintainer.

Senior maintainer review of PR #44 ("Add optional SpinGlassPEPS backend"), diffed against the merge-base of origin/feature/issue-37-backend-interface and the PR head (009bcd1). Reproduced totals: 10 files, +646/-5. Only issues introduced by #44 itself are flagged; the #41-#43 base chain is treated as inherited.

Process note: I am the PR author, so GitHub only permits an event = COMMENT from this account. A formal approve/request-changes must come from another maintainer.

Test execution: the review sandbox denied julia and git (including with sandbox disabled), so I could not run the suite locally. Findings are from static review of the head sources plus CI. CI is green at the head SHA across all 12 check-runs (Julia 1/lts/pre x ubuntu/macOS/windows, build, docs, codecov). Caveat: CI ran with the stack cut from old main, and the optional SpinGlassPEPS path is @test_skip-ped because the component packages are not installed in CI, so the real PEPS solve and decode path is NOT exercised by any automated test.

Optionality mechanism (verified correct): the SpinGlass components are declared under [weakdeps] + [extensions] (TenSolverSpinGlassPEPSExt), not hard [deps]. Compat bounds are set (SpinGlassEngine 1.6, SpinGlassNetworks 1.4, SpinGlassTensors 1.3; QUBOTools widened to "0.10, 0.11"). When the extension is not loaded, PEPSBackend solves throw a clear ArgumentError naming the three packages and pointing to backend = :dmrg (covered by test/peps_backend.jl). This satisfies issue #38's "do not add SpinGlassPEPS as mandatory deps" requirement and the packaging-option-1 (extension) preference. Ordinary installs keep the existing dependency footprint.

Mapping correctness (static): _ising_instance builds the SpinGlassNetworks dict from model.h (diagonal) and the upper-triangle of model.J, which matches IsingModel's objective offset + sum(h_i s_i) + sum_{i<j} J_ij s_i s_j. The constant offset is intentionally dropped from the instance and reintroduced by recomputing the final energy via ising_energy(model, spins), so returned energies are in TenSolver's original convention. Decode uses decode_potts_hamiltonian_state then spin_to_bool. Square/king super_square_lattice clustering and the SquareSingleNode/KingSingleNode{GaugesEnergy} networks follow the issue's implementation sketch. Multi-spin-per-site is handled in _check_layout_edges by skipping intra-cell couplings (ci == cj). I did not find a correctness bug in the mapping by inspection, but see the no-coverage caveat below.

1. Blocking

None introduced by this PR. The mapping/decoding code is plausible but completely unexercised by CI (the only test that would run it is skipped). I am not raising this to Blocking because issue #38 explicitly anticipates this as packaging-option-3 ("clearly experimental branch-only prototype if dependency/version constraints block an extension") and the PR keeps the PEPS types unexported and out of the public API docs. But it is the single largest risk: 244 lines of extension logic ship with zero executed-path coverage. See Questions.

2. Nonblocking

Nonblocking: sample/prob/in on PEPSSolution operate on the deduplicated retained states, and _deduplicated_records keeps only the first record per Boolean state. Duplicate states' probability mass is therefore dropped rather than summed, and sample's cumulative-weight draw uses the un-renormalized retained probabilities. This is acceptable for "retained states" semantics but is a latent fidelity gap if two PEPS records decode to the same Boolean vector; a one-line comment or summing duplicates in prob would make the intent explicit.

Nonblocking: in TenSolver._solve_ising the per-transformation loop uses try ... finally clear_memoize_cache() with no catch. If any single transformation in transformations = :all throws inside SpinGlassEngine, the whole solve aborts even when other transformations would have succeeded. Issue #38 frames transformations as a set "to try"; consider catching and recording per-transform failures so one bad transform does not fail the solve. Not blocking since the default-path correctness is unaffected.

Nonblocking: the cutoff keyword (always injected by minimize(...; cutoff = 1e-8)) is silently absorbed by the extension's _solve_ising(...; cutoff = nothing, ...) and ignored. That is the right behavior for a DMRG-specific knob, but the extension's strict "Unsupported PEPS backend keyword(s)" guard relies on cutoff (and verbosity) being explicitly captured. A short comment noting that cutoff is intentionally accepted-and-ignored would prevent a future refactor from accidentally routing it into the kwargs guard and breaking minimize(Q; backend = PEPSBackend(...)).

3. Questions

Question: Closes #38. Issue #38's acceptance criteria require "A PEPS-backed direct API works for at least one small structured grid case on CPU" and "Exact small tests prove objective consistency." Because the SpinGlass stack does not resolve in the TenSolver environment (documented in the PR body and in docs/src/spinglasspeps_integration.md), that acceptance test is skipped, not passing. The core scaffolding, error handling, and DMRG-path solve_ising tests do pass. Given the real PEPS solve was never executed end-to-end, should this be Refs #38 (keeping #38 open until the component stack resolves and the gated test actually runs) rather than Closes #38? As written, merging would auto-close #38 while its primary acceptance criterion remains unverified.

Question: export Solution is added to src/TenSolver.jl. Solution was previously unexported (documented via TenSolver.Solution in api.md). Exporting it is a public-API surface change that is unrelated to the SpinGlassPEPS backend goal of this PR. Is this intentional and in scope for #44, or stack drift that belongs in a separate change?

4. Tests run and outcomes

Local execution blocked: julia and git were both denied by the review sandbox (confirmed even with sandbox disabled; not retried). Verified statically from head sources via gh pr diff/gh api contents. CI: all 12 check-runs success at head f3c4db1 (build, docs deploy, codecov patch+project, and the 9 Julia matrix jobs). The optional SpinGlassPEPS test is skipped in CI (Base.find_package returns nothing for the components), so the actual PEPS solve/decode path has no executed coverage anywhere.

5. Merge-readiness

Not mergeable yet, primarily due to the stacked-base dependency, not code defects. PR #44's base is feature/issue-37-backend-interface (PR #43, still OPEN), which itself chains through #42 -> #41 -> main. #44 cannot merge until #41, #42, and #43 merge first. Additionally, the whole #41..#46 stack was cut from old main (ca27880); current origin/main is 9a3dd5c (v0.2.0, with #49/#53/#54 merged), so the stack is stale and carries rebase/conflict risk against current main (Project.toml compat and exports are likely touch points). CI green here was computed on the stale base. No code changes are required from a correctness standpoint; the nonblocking items and the Closes #38 vs Refs #38 question above are the substantive follow-ups.

I would not merge this until the stacked base chain (#41-#43) merges and the stack is rebased onto current main; the Closes #38 linkage should also be reconsidered given the PEPS solve path is not exercised by CI.